-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[extension/azure_encoding] Add support for Administrative, Alert, Autoscale, Policy, Security, ServiceHealth, and ResourceHealth log categories. #45699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@Fiery-Fenix, I’d love to get your eyes on this as well, whenever you have a chance. |
# Conflicts: # extension/encoding/azureencodingextension/internal/unmarshaler/logs/README.md # extension/encoding/azureencodingextension/internal/unmarshaler/logs/category.go # extension/encoding/azureencodingextension/internal/unmarshaler/logs/helpers.go
Use the tranlator/azurelogs test document
Use the translator/azurelogs test document
Use the translator/azurelogs test document
Use the translator/azurelogs test document
Use the translator/azurelogs test document
95b92f4 to
4028542
Compare
| // log record. | ||
| // Extract common claim fields | ||
|
|
||
| if iss := id.Claims[identityClaimIssuer]; iss != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for readability it would be better to use map, except for timestamps of course
Something like:
claimAttributesMap := map[string]string{
"iss": attributeIdentityClaimsIssuer,
....
}
for k, v := range claimAttributesMap {
if value, has := id.Claim[k]; has {
unmarshaler.AttrPutStrIf(attrs, v, value)
}
}
| @@ -85,3 +88,12 @@ func attrPutTLSProtoIf(attrs pcommon.Map, securityProtocol string) { | |||
| unmarshaler.AttrPutStrIf(attrs, string(conventions.TLSProtocolNameKey), name) | |||
| unmarshaler.AttrPutStrIf(attrs, string(conventions.TLSProtocolVersionKey), version) | |||
| } | |||
|
|
|||
| // parseUnixTimestamp parses a Unix timestamp string (seconds since epoch) and returns a time.Time | |||
| func parseUnixTimestamp(s string) (time.Time, error) { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about test for this function?
| // ------------------------------------------------------------ | ||
|
|
||
| if id.Authorization != nil { | ||
| attrs.PutStr(attributeIdentityAuthorizationScope, id.Authorization.Scope) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we interested in empty values for this attributes?
If no - we should probably use unmarshaler.AttrPutStrIf(attrs, _attribute_, _value_) to avoid empty values for output attributes, if any
| ) | ||
|
|
||
| // Constants for Identity > claims | ||
| const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it make sense to extract all Identity parsing code into separate file? As it's already 160+ lines of code
|
Also I think it's better to place all test data (input json files and expected yaml files) into single folder that correlates with name of code file that is actually parsing them, like it done for other |
Description
This PR brings over the remaining Activity log categories from
translator/azurelogs, aligning this component with the changes made in #44871.New log categories:
Testing
We added test cases for each category, leveraging the same log events found in translator/azurelogs for consistency.
Documentation
Added a section covering Identity (including its major subfields) along with dedicated sections for each new log category.